bpo-42972: Fix sqlite3 traverse/clear#26452
Conversation
|
Whenever you have time, Pablo & Victor. |
bce1100 to
2e7f768
Compare
2e7f768 to
62bbb5c
Compare
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 62bbb5c 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
|
PPC64LE Fedora Stable Refleaks PR: buildbot/AMD64 Windows8.1 Refleaks PR:
|
vstinner
left a comment
There was a problem hiding this comment.
LGTM. I checked that all Python objects are visited in traverse functions, and that clear functions clear all objects visited in traverse functions.
The coding style issue is not worth to require another revision.
| @@ -42,9 +42,9 @@ row_clear(pysqlite_Row *self) | |||
| static int | |||
| row_traverse(pysqlite_Row *self, visitproc visit, void *arg) | |||
There was a problem hiding this comment.
nitpick: row.c is the only file where clear is declared before traverse.
|
Thanks @erlend-aasland for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
(cherry picked from commit d1124b0) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
|
GH-26461 is a backport of this pull request to the 3.10 branch. |
|
@erlend-aasland: While reviewing traverse/clear functions, I noticed surprising code in pysqlite_cache_dealloc(): PyObject_GC_UnTrack() is not called if pysqlite_cache_init() failed, IMO it's a bug: the object is tracked in new, not in init. Note: pysqlite_cache_init() doesn't use Py_SETREF() or Py_CLEAR() and so leaks references if called manually twice. Well, nobody should do that ;-) |
Yes, those are unfortunate issues :) However, they'll soon (hopefully) begone! Hint: #24203 😃 (You're welcome to take a look at that, if you find time) |
|
|
Thanks for reviewing, @vstinner ! |
GH-26104 had some issues:
Py_tp_cleariso.Py_tp_deallocThis PR fixes these issues.
https://bugs.python.org/issue42972